packages/suite/src/components/wallet/Fees/CustomFee.tsx (1)

53-53: ⚠️ Potential issue

Add safety checks to getCurrentFee function.

The function needs validation to handle edge cases safely.

-const getCurrentFee = (levels: FeeLevel[]) => `${levels[levels.length > 2 ? 1 : 0].feePerUnit}`;
+const getCurrentFee = (levels: FeeLevel[]): string => {
+    if (!levels?.length) return '0';
+    const index = levels.length > 2 ? 1 : 0;
+    return `${levels[index]?.feePerUnit ?? '0'}`;
+};

packages/suite/src/components/wallet/Fees/Fees.tsx (1)

86-145: 🛠️ Refactor suggestion

Improve error handling and type safety in buildFeeOptions.

The function needs better error handling and type safety improvements.

 const buildFeeOptions = (
     levels: FeeLevel[],
     networkType: NetworkType,
     symbol: NetworkSymbol,
     translationString: TranslationFunction,
     composedLevels?: PrecomposedLevels | PrecomposedLevelsCardano,
 ) => {
+    if (!levels?.length) {
+        console.warn('No fee levels provided');
+        return [];
+    }
+
     const filteredLevels = levels.filter(level => level.label !== 'custom');
 
     const getNetworkAmount = (level: FeeLevel) => {
         const transactionInfo = composedLevels?.[level.label];
         const hasTransactionInfo =
             transactionInfo !== undefined && transactionInfo.type !== 'error';
-        const networkAmount = hasTransactionInfo
-            ? formatNetworkAmount(transactionInfo.fee, symbol)
-            : null;
+        const networkAmount = hasTransactionInfo && transactionInfo.fee
+            ? formatNetworkAmount(transactionInfo.fee, symbol)
+            : null;
         // Needed only for Solana because of fee estimation on compose Tx
-        const fee = hasTransactionInfo ? transactionInfo.fee : level.feePerTx;
+        const fee = hasTransactionInfo && transactionInfo.fee ? transactionInfo.fee : level.feePerTx;
 
         return { networkAmount, fee };
     };

suite-common/wallet-config/src/networksConfig.ts (1)

129-129: LGTM! Consider adding documentation.

The settlementLayer property is correctly applied to Ethereum L2 networks (Arbitrum, Base, Optimism). This helps distinguish true L2s from sidechains.

Consider adding a comment in the network configuration to document why certain networks have the settlementLayer property while others (like Polygon and BSC) don't, to prevent future confusion.

Also applies to: 154-154, 179-179

packages/product-components/src/components/CoinLogo/CoinLogo.tsx (1)

84-85: LGTM! Consider a minor optimization.

The change improves error handling by using getNetworkOptional and correctly handles the new settlementLayer property.

Consider destructuring the network object to make the code more concise:

-const network = getNetworkOptional(symbol);
-const networkSymbol = network ? network.settlementLayer || symbol : symbol;
+const { settlementLayer } = getNetworkOptional(symbol) ?? {};
+const networkSymbol = settlementLayer ?? symbol;

packages/suite/src/components/wallet/Fees/FeeDetails.tsx (1)

252-255: Consider adding a loading state for Solana fees.

The comment suggests future Solana-specific handling. Consider adding a loading state while fees are being fetched.

+    const [isLoading, setIsLoading] = useState(false);
+
     if (!feeOptions?.length) return null;

     const feeOption = feeOptions[0]; // in the future Solana should have it's own Details component
-    const feeAmount = networkType === 'solana' ? feeOption.feePerTx : feeOption.feePerUnit;
+    const feeAmount = networkType === 'solana' 
+        ? isLoading ? 'Loading...' : feeOption.feePerTx 
+        : feeOption.feePerUnit;

packages/suite/src/components/wallet/Fees/Fees.tsx (2)

219-235: Add ARIA labels for better accessibility.

The SelectBar component needs proper ARIA labels for better accessibility.

                     <SelectBarWrapper>
                         <SelectBar
                             orientation="horizontal"
                             selectedOption={isCustomFee ? 'custom' : 'normal'}
+                            aria-label="Fee calculation mode"
                             options={[
-                                { label: 'Standard', value: 'normal' },
-                                { label: 'Advanced', value: 'custom' },
+                                { label: 'Standard', value: 'normal', 'aria-label': 'Standard fee calculation' },
+                                { label: 'Advanced', value: 'custom', 'aria-label': 'Advanced fee calculation' },
                             ]}
                             onChange={() => {
                                 changeFeeLevel(isCustomFee ? 'normal' : 'custom');
                                 setIsCustomFee(!isCustomFee);
                             }}
                             isFullWidth
                         />
                     </SelectBarWrapper>

---

271-329: Add loading states during fee calculations.

The custom fee section should show loading states during fee calculations to improve user experience.

+    const [isCalculating, setIsCalculating] = useState(false);
+
     <AnimatePresence>
         {isCustomFee && (
             <motion.div
                 key="customFee"
                 initial={{ opacity: 0, height: 0 }}
                 animate={{ opacity: 1, height: 'auto' }}
                 exit={{ opacity: 0, height: 0 }}
                 transition={{
                     opacity: { duration: 0.15, ease: motionEasing.transition },
                     height: { duration: 0.2, ease: motionEasing.transition },
                     marginTop: { duration: 0.25, ease: motionEasing.transition },
                 }}
                 style={{ overflow: 'hidden' }}
             >
                 <CustomFee
                     control={control}
                     networkType={networkType}
                     feeInfo={feeInfo}
                     errors={errors}
                     register={register}
                     getValues={getValues}
                     setValue={setValue}
+                    onCalculationStart={() => setIsCalculating(true)}
+                    onCalculationEnd={() => setIsCalculating(false)}
                     composedFeePerByte={
                         (transactionInfo as PrecomposedTransactionFinal)?.feePerByte
                     }
                 />
                 <Column>
                     <Divider margin={{ bottom: spacings.md }} />
                     <Row
                         gap={spacings.sm}
                         alignItems="baseline"
                         justifyContent="space-between"
                     >
                         <Text variant="tertiary" typographyStyle="hint">
                             <Translation id="FEE" />:
                         </Text>
-                        {networkAmount && (
+                        {isCalculating ? (
+                            <Text variant="tertiary">Calculating...</Text>
+                        ) : networkAmount && (
                             <Row gap={spacings.xxs}>

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/suite/src/components/wallet/Fees/CustomFee.tsx (1)

53-58: ⚠️ Potential issue

Add safety checks to getCurrentFee function.

The function needs validation to handle edge cases safely:

• Empty arrays
• Missing feePerUnit values
• Invalid levels array

Apply this diff to improve safety:

-const getCurrentFee = (levels: FeeLevel[]) => {
+const getCurrentFee = (levels: FeeLevel[]): string => {
+    if (!levels?.length) return '0';
     const middleIndex = Math.floor((levels.length - 1) / 2);
-    return levels[middleIndex].feePerUnit;
+    return levels[middleIndex]?.feePerUnit ?? '0';
};

packages/suite/src/components/wallet/Fees/FeeDetails.tsx (7)

80-114: Consider memoizing the onClick handler.

The component is well-structured, but the onClick handler could be optimized to prevent unnecessary re-renders.

+    const handleClick = React.useCallback(() => {
+        setSelectedLevelOption(value);
+        changeFeeLevel(value);
+    }, [value, setSelectedLevelOption, changeFeeLevel]);
+
     <RadioCard
-        onClick={() => {
-            setSelectedLevelOption(value);
-            changeFeeLevel(value);
-        }}
+        onClick={handleClick}
         isActive={isSelected}
     >

---

145-146: Use stable keys for mapped components.

Using array indices as keys can lead to performance issues and potential bugs during re-renders when items are added/removed/reordered.

-                {feeOptions?.map((fee, index) => (
-                    <FeeCard
-                        key={index}
+                {feeOptions?.map(fee => (
+                    <FeeCard
+                        key={fee.value}

---

167-167: Use consistent null coalescing operator.

Replace logical OR with null coalescing for consistent null/undefined handling.

-                                amount={fee?.networkAmount || ''}
+                                amount={fee?.networkAmount ?? ''}

-                                    feeInfo.blockTime * (fee?.blocks ?? 0) * 60,
+                                    feeInfo.blockTime * (fee.blocks ?? 0) * 60,

Also applies to: 159-159

---

203-211: Extract fee formatting logic to a utility function.

The fee formatting logic is complex and could be reused elsewhere. Consider moving it to a utility function.

+const formatEthereumFee = (feePerUnit: string | undefined, isMainnet: boolean) => {
+    const num = Number(feePerUnit) ?? 0;
+    const numOfDecimalPlaces = isMainnet ? 2 : 4;
+    const multiplier = Math.pow(10, numOfDecimalPlaces);
+    return (Math.ceil(num * multiplier) / multiplier).toFixed(numOfDecimalPlaces);
+};
+
-    const formatFeePerUnit = (feePerUnit?: string) => {
-        const num = Number(feePerUnit) || 0;
-        const numOfDecimalPlaces = isMainnet ? 2 : 4;
-        const multiplier = Math.pow(10, numOfDecimalPlaces);
-        return (Math.ceil(num * multiplier) / multiplier).toFixed(numOfDecimalPlaces);
-    };

---

217-218: Apply consistent array handling and null coalescing.

Similar to BitcoinDetails, use stable keys and consistent null handling.

-                    {feeOptions?.map((fee, index) => (
-                        <FeeCard
-                            key={index}
+                    {feeOptions?.map(fee => (
+                        <FeeCard
+                            key={fee.value}

-                                    amount={fee.networkAmount || ''}
+                                    amount={fee.networkAmount ?? ''}

-                                    {formatFeePerUnit(fee?.feePerUnit)}
+                                    {formatFeePerUnit(fee.feePerUnit)}

Also applies to: 231-231, 238-238

---

260-261: Use more descriptive variable names.

The variable names could be more descriptive to improve code readability.

-    const feeOption = feeOptions[0];
-    const feeAmount = networkType === 'solana' ? feeOption.feePerTx : feeOption.feePerUnit;
+    const defaultFeeOption = feeOptions[0];
+    const displayedFeeAmount = networkType === 'solana' ? defaultFeeOption.feePerTx : defaultFeeOption.feePerUnit;

---

260-260: Track TODO about Solana-specific component.

The comment indicates a future Solana-specific component is needed.

Would you like me to create an issue to track the implementation of a Solana-specific fee details component?

packages/suite/src/components/wallet/Fees/CustomFee.tsx (1)

42-51: LGTM! Simplified props interface.

The removal of the unused changeFeeLimit prop simplifies the component's interface. Fee limit changes are now handled through the useForm hook's control, which is a cleaner approach.

packages/suite/src/components/wallet/Fees/Fees.tsx (4)

54-61: Add JSDoc documentation to the FeeOption type.

Adding documentation would improve code maintainability by explaining the purpose of each field and when they are expected to be present.

+/**
+ * Represents a fee option in the fee selector.
+ * @property label - Display text for the fee option
+ * @property value - Identifier for the fee level
+ * @property blocks - (Bitcoin only) Expected number of blocks until confirmation
+ * @property feePerUnit - Fee rate per unit of transaction size
+ * @property networkAmount - Formatted total fee amount in network currency
+ * @property feePerTx - (Solana only) Fixed fee per transaction
+ */
 export type FeeOption = {
     label: React.ReactNode;
     value: FeeLevel['label'];
     blocks?: number;
     feePerUnit?: string;
     networkAmount?: string | null;
     feePerTx?: string;
 };

---

63-67: Consider using theme-based responsive width.

The hardcoded width might not be optimal for all screen sizes. Consider using theme-based responsive width or percentage-based width for better adaptability.

 const SelectBarWrapper = styled.div`
-    width: 239px;
+    width: min(239px, 100%);
     justify-self: end;
     margin-top: ${spacingsPx.xs};
 `;

---

317-321: Enhance error banner with more detailed information.

The error banner could provide more context about the error and possible solutions.

 {error && (
     <Banner icon margin={{ top: spacings.sm }} variant="destructive">
-        {error.message}
+        <Column>
+            <Text>{error.message}</Text>
+            {error.type === 'invalidFee' && (
+                <Text variant="hint">
+                    <Translation id="TR_FEE_ERROR_HINT" />
+                </Text>
+            )}
+        </Column>
     </Banner>
 )}

---

231-238: Consider extracting animation configuration to constants.

The animation durations and configurations are hardcoded. Consider extracting them to constants for consistency and reusability.

+const ANIMATION_CONFIG = {
+    opacity: { duration: 0.15, ease: motionEasing.transition },
+    height: { duration: 0.2, ease: motionEasing.transition },
+    marginTop: { duration: 0.25, ease: motionEasing.transition },
+} as const;

 <motion.div
     key="feeDetails"
     initial={{ opacity: 0, height: 0 }}
     animate={{ opacity: 1, height: 'auto' }}
     exit={{ opacity: 0, height: 0 }}
-    transition={{
-        opacity: { duration: 0.15, ease: motionEasing.transition },
-        height: { duration: 0.2, ease: motionEasing.transition },
-        marginTop: { duration: 0.25, ease: motionEasing.transition },
-    }}
+    transition={ANIMATION_CONFIG}

packages/product-components/src/components/FeeRate/FeeRate.tsx (1)

15-16:

❓ Verification inconclusive

Consider network-specific fee formatting.

While the current implementation distinguishes between Bitcoin and other networks, it might be beneficial to:

1. Extract the formatting logic to a utility function for better maintainability
2. Add network-specific formatting rules for better UX across different networks

Consider this approach:

-            {networkType === 'bitcoin' ? fee.toFixed(2) : fee.toString()} 
-            {getFeeUnits(networkType)}
+            {formatNetworkFee(fee, networkType)} {getFeeUnits(networkType)}

Add a new utility function:

const formatNetworkFee = (fee: BigNumber, networkType: NetworkType): string => {
  switch (networkType) {
    case 'bitcoin':
      return fee.toFixed(2);
    case 'solana':
      // Format Solana fees with appropriate precision
      return fee.toFixed(9);
    // Add cases for other networks as needed
    default:
      return fee.toString();
  }
};

Let's check the current fee formatting practices in the codebase:

---

🏁 Script executed:

#!/bin/bash
# Search for fee formatting patterns across the codebase
rg -A 2 'toFixed|toString' --type typescript

Length of output: 81

---

Enhance Fee Formatting Logic in FeeRate Component

The current implementation uses a conditional format—toFixed(2) for Bitcoin and toString() for other networks. To improve maintainability and consistency across networks, please consider:

• Extracting Fee Formatting Logic: Move the network-specific formatting into a utility function (e.g., formatNetworkFee).
• Extending Network-Specific Rules: For example, handle Solana fees with higher precision (e.g., using toFixed(9)) and add cases for other networks as needed.

Example diff:

-            {networkType === 'bitcoin' ? fee.toFixed(2) : fee.toString()} 
-            {getFeeUnits(networkType)}
+            {formatNetworkFee(fee, networkType)} {getFeeUnits(networkType)}

And a utility function suggestion:

const formatNetworkFee = (fee: BigNumber, networkType: NetworkType): string => {
  switch (networkType) {
    case 'bitcoin':
      return fee.toFixed(2);
    case 'solana':
      // Format Solana fees with appropriate precision
      return fee.toFixed(9);
    // Add cases for additional networks as needed
    default:
      return fee.toString();
  }
};

Please verify that this updated logic aligns with fee formatting throughout the codebase—especially given that our initial search for formatting patterns using rg -A 2 'toFixed|toString' --type typescript produced an error. To re-check fee formatting across TypeScript files, please run the following script:

Review the results carefully to ensure there are no overlooked patterns before merging.

---

🏁 Script executed:

[run_scripts]
#!/bin/bash
# Re-run fee formatting search using a glob pattern for .ts and .tsx files
rg -A 2 'toFixed|toString' --glob '*.{ts,tsx}'

Length of output: 72852

---

Enhance Fee Formatting Logic in FeeRate Component

The current inline formatting handles Bitcoin fees with a fixed two-decimal format and defaults to native formatting for other networks. For improved consistency and maintainability—especially as support for additional networks (e.g. Solana) may require tailored precision—it would be beneficial to extract this logic into a dedicated utility function.

Suggestions:

• Replace the inline conditional:

      {networkType === 'bitcoin' ? fee.toFixed(2) : fee.toString()} 
      {getFeeUnits(networkType)}

  with a call to a new utility function:

  -            {networkType === 'bitcoin' ? fee.toFixed(2) : fee.toString()} 
  -            {getFeeUnits(networkType)}
  +            {formatNetworkFee(fee, networkType)} {getFeeUnits(networkType)}

• Implement a utility function, for example:

  const formatNetworkFee = (fee: BigNumber, networkType: NetworkType): string => {
    switch (networkType) {
      case 'bitcoin':
        return fee.toFixed(2);
      case 'solana':
        // Example: use higher precision for Solana fees
        return fee.toFixed(9);
      // Extend with additional networks as needed
      default:
        return fee.toString();
    }
  };

Given that our recent search across TypeScript files confirms the inline formatting is applied in this component and similar patterns are dispersed elsewhere in the codebase, please verify that introducing this utility function aligns with overall fee formatting conventions and UX across supported networks.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: tomasklim
PR: trezor/trezor-suite#16702
File: packages/suite/src/components/wallet/Fees/FeeDetails.tsx:60-79
Timestamp: 2025-02-14T21:36:51.801Z
Learning: The Trezor Suite application has proper browser compatibility and support for ResizeObserver, making additional error handling unnecessary.

packages/components/src/components/InfoItem/InfoItem.tsx (1)

89-89: Consider using theme variables for the height property.

While adding a fixed height helps maintain consistent layout as per the fees redesign, consider:

1. Using theme variables (like spacings) instead of hard-coded values for better maintainability.
2. Ensuring the height accommodates different font sizes and localized content.

-                    height={24}
+                    height={spacings.xl}

Please verify this change against the Figma design mockup mentioned in the PR description to ensure the spacing aligns with the design system.

packages/suite/src/components/wallet/Fees/FeeDetails.tsx (2)

145-186: Improve array handling and key usage.

The component could benefit from more robust array handling and better key usage.

Apply this diff to improve the implementation:

-                {feeOptions?.map((fee, index) => (
+                {feeOptions?.length > 0 && feeOptions.map(fee => (
                     <FeeCard
-                        key={index}
+                        key={fee.value}
                         value={fee.value}

---

260-260: Consider implementing a dedicated Solana component.

The comment indicates that Solana should have its own Details component in the future.

Would you like me to help create a dedicated Solana component implementation?

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: tomasklim
PR: trezor/trezor-suite#16702
File: packages/suite/src/components/wallet/Fees/FeeDetails.tsx:60-79
Timestamp: 2025-02-14T21:36:51.801Z
Learning: The Trezor Suite application has proper browser compatibility and support for ResizeObserver, making additional error handling unnecessary.